Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Substitute complicated PNLF _UID implementation #91

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

zhen-zen
Copy link
Contributor

@zhen-zen zhen-zen commented Sep 26, 2021

Current SSDT-PNLF relies on PCI and MMIO access from ACPI space. However, only proper _UID is required for most cases. Workarounds like BacklightRegistersFix are also available in WhateverGreen.

By moving _UID identification to WhateverGreen, it's possible to make PNLF device a simple stub matched by AppleIntelPanel. Purposed solution is to use a ACPI method SUID that accepts desired _UID from WhateverGreen. It might need extra work to replace it by patching AppleIntelPanel::start which evaluates _UID.

Sample DSDT attached in the commit but will remove later.

Tested on 8086:9b41 (CML, faked to 3e9b) and 8086:5927 (KBL-R, faked to 5926).

@@ -375,6 +376,47 @@ void WEG::processKext(KernelPatcher &patcher, size_t index, mach_vm_address_t ad
return;
}

uint32_t processUID(uint32_t deviceid) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it good to just replace it with CPUInfo::CpuGeneration ?

@Lorys89
Copy link
Contributor

Lorys89 commented Sep 26, 2021

it's a great idea and I'm available to test over multiple generations in my hacks.

@Sher1ocks
Copy link
Contributor

is it support snow leopard and lion?

@vit9696
Copy link
Collaborator

vit9696 commented Sep 26, 2021

Thanks!

  • Is it reliable? I am worried about data races between updating UID by ourselves and macOS reading it.
  • CPU generation feels the right way to do the UID detection to me, but it will need more thorough testing and possibly updates to the CPU detection code.
  • What are the downsides of the SSDT route besides the complexity?

@zhen-zen
Copy link
Contributor Author

is it support snow leopard and lion?

The implementation should be compatible with older OS but currently I don't have such platforms for testing.

Also @Lorys89 mentioned failure on his Haswell laptop. Will discuss it below.

Thanks!

  • Is it reliable? I am worried about data races between updating UID by ourselves and macOS reading it.

I put it in processBuiltinProperties() which is called during init(). So it's executed before any Framebuffer and AppleBacklight. Also, all init() happens before AppleIntelPanel::start reads _UID with evaluateInteger.

I don't know how to figure out the class structure in IOKit, here is my best guess:
bad-little-cat

And before the fix kicks in, ACPI driver will first evaluate every _UID and then publish it in ioreg. That's what the latter cosmetic part does.

  • CPU generation feels the right way to do the UID detection to me, but it will need more thorough testing and possibly updates to the CPU detection code.

I agree it's the correct way, but sometimes iGPUs like UHD620 might be wrongly classified. We to find a way out for some corner cases. However I suppose It could be deducted from current list? Or maybe by checking which Framebuffer is loaded?

  • What are the downsides of the SSDT route besides the complexity?

The SSDT applies fixes for some early generations. However, the variant targeting CFL does nothing while we also have a fix in WEG. I also doubt if the extra work is needed for SKL+. It's better to keep the fix in a single place for less interference in further debugging.

If only the PWMMAX is required for override, or the HSW case, I suppose we can implement that in WEG since we also has access via readRegister32. Or maybe something like https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/i915/display/intel_panel.c#L835 .

This new feature should do nothing with old SSDT that has no SUID method. We can revert the recent CFL+ change since we don't need it being merged, keeping only the Windows workaround. And add the stub in place of current PNLFCFL one. This might leave us buffer for related issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants